Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ux: add warning when cabal reads from no index cache #10258

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

taimoorzaeem
Copy link
Contributor

@taimoorzaeem taimoorzaeem commented Aug 15, 2024

Fixes #9136.

Since, I am a new contributor, I didn't follow the provided PR template and wrote what I thought would be best in this PR. Let me know if following the provided PR template is necessary.

  • Add warning in code
  • Add tests
  • Add QA Notes
  • Conform to coding conventions

@andreabedini Please check if that is the right place for the warning and let me know about changes. I'll work on adding the tests next.

@ulysses4ever
Copy link
Collaborator

@taimoorzaeem great work, thanks!

Could you, please, add manual-QA notes in the PR description? This would be a step-by-step guide showing what it takes to see the effect of your patch locally. This will start with build cabal from your branch (obvious). But then, what do I do to see the warning? Step by step. You could attach any sample files that could help to set up a no-index repository etc. Example of MQA notes is in the description of #9200

@ulysses4ever
Copy link
Collaborator

You'll also have to make fourmolu and whitespace checkers happy (see the github action failures linked from this page).

@ulysses4ever
Copy link
Collaborator

Would be great to add a test. See for guidance: https://github.com/haskell/cabal/blob/master/cabal-testsuite/README.md#how-to-write

@taimoorzaeem taimoorzaeem changed the title UX: add warning when using noindex.cache ux: add warning when cabal reads from no index cache Aug 16, 2024
@taimoorzaeem
Copy link
Contributor Author

@ulysses4ever @andreabedini When exactly does cabal create a local .cache file? Are there any example of such test cases that I can follow?

@taimoorzaeem taimoorzaeem marked this pull request as draft August 16, 2024 15:14
@ulysses4ever
Copy link
Collaborator

@taimoorzaeem I don't know the answer to your last question from the top of my head, sadly. I could try to dig over the weekend maybe. But in the meantime: can you locally observe the effect of your patch? In other words, the message you added --- can you make cabal built with this patch print this message? If so, please, post the instructions for how to do that.

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Aug 22, 2024

37 tests affected by the current change (they fail now), but most (all?) of them are seemingly unrelated:

562 tests, 14 skipped, 0 unexpected passes, 37 unexpected fails.
UNEXPECTED FAIL: PackageTests/ShowBuildInfo/Complex/single.test.hs PackageTests/HaddockArgs/quickjump.test.hs PackageTests/HaddockArgs/hoogle.test.hs PackageTests/ConditionalAndImport/cabal.test.hs PackageTests/HaddockProject/haddock-project.test.hs PackageTests/NewFreeze/Flags/new_freeze.test.hs PackageTests/NewFreeze/FreezeFile/new_freeze.test.hs PackageTests/NewFreeze/BuildTools/new_freeze.test.hs PackageTests/BuildTargets/UseLocalPackageForSetup/use-local-package-as-setup-dep.test.hs PackageTests/BuildTargets/UseLocalPackage/use-local-version-of-package.test.hs PackageTests/Backpack/Includes3/cabal-repo.test.hs PackageTests/Backpack/T6385/cabal.test.hs PackageTests/Regression/T9756/cabal.test.hs PackageTests/Regression/T9640/cabal.test.hs PackageTests/NewHaddock/DisableDoc/cabal.test.hs PackageTests/NewHaddock/HaddockOutput/HaddockOutputCmd/cabal.test.hs PackageTests/NewHaddock/HaddockOutput/HaddockOutputConfig/cabal.test.hs PackageTests/NewHaddock/ImplyDependencies/cabal.test.hs PackageTests/OfflineFlag/offlineFlag.test.hs PackageTests/Outdated/outdated.test.hs PackageTests/Outdated/outdated-project-file.test.hs PackageTests/Outdated/outdated_freeze.test.hs PackageTests/VersionPriority/3-web.test.hs PackageTests/VersionPriority/2-local.test.hs PackageTests/VersionPriority/1-web.test.hs PackageTests/VersionPriority/1-local.test.hs PackageTests/VersionPriority/0-local.test.hs PackageTests/VersionPriority/2-web.test.hs PackageTests/Get/OnlyDescription/cabal.test.hs PackageTests/ExtraPackages/cabal.test.hs PackageTests/CopyHie/cabal.test.hs PackageTests/HaddockBuildDepends/cabal.test.hs PackageTests/Freeze/enable-benchmarks.test.hs PackageTests/Freeze/disable-tests.test.hs PackageTests/Freeze/disable-benchmarks.test.hs PackageTests/Freeze/freeze.test.hs PackageTests/Freeze/enable-tests.test.hs

@taimoorzaeem
Copy link
Contributor Author

can you locally observe the effect of your patch? In other words, the message you added --- can you make cabal built with this patch print this message? If so, please, post the instructions for how to do that.

No, I am not able to observe the effect locally. That's because I am not sure when exactly cabal create a local no index cache. The warning message is only supposed to be printed when this cache file is created by cabal. Since I am not a much advanced user of cabal, I would like some you folks' help. @andreabedini Could you help out on how I should go about testing/qa it locally?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local no-index repositories cache leads to bad UX
2 participants